👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

🔔 1st Reminder

Hey @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

we so far failed to establish a single standard authentication mechanism

Isn't this mostly/almost entirely because we failed to ever actually offer any authentication mechanisms? The only auth mechanism currently implemented on the server-side (which was only recently added) is LNURL-Auth, which isn't even freestanding - it requires a second server somewhere that we don't provide. On the client side we allow for a "just give us the header" implementation to do fully-custom things, but that doens't help all that much either.

If we'd provided a standard way to do authentication from day one, I have to assume people would have used it.

Could you expand on why you couldn't just use the existing JWT auth for your usecase?

The authentication scheme here is totally different from what already exists - instead of having the wallet contact a server and ask for the server to sign off on the client accessing data for its user-id, this just allows the client to access its data by signing a message. This sidesteps having to implement a second server entirely, not to mention reduces complexity and dependencies.

Isn't this mostly/almost entirely because we failed to ever actually offer any authentication mechanisms?

Well, no, at the time there was some push-back to the idea of featuring a particular default/preferred authentication scheme, as some people were of the opinion that that should be completely left to the user as they would want to integrate with whatever authentication infrastructure they'd already have in place.

The only auth mechanism currently implemented on the server-side (which was only recently added) is LNURL-Auth

Also not quite accurate, as we have users that have been running their LNURL-auth implementation for quite some time. But yes, we only recently finally made progress to add support to vss-server itself.

it requires a second server somewhere that we don't provide

I was under the impression that we concluded to finally change that and provide a default implementation?

If we'd provided a standard way to do authentication from day one, I have to assume people would have used it.

Yeah, see above, I had always considered us not providing a default auth mechanism a failure on our end that unnecessarily risks protocol fragmentation. So fully agree we should close that gap.

The authentication scheme here is totally different from what already exists - instead of having the wallet contact a server and ask for the server to sign off on the client accessing data for its user-id,

Not sure I'm completely following, AFAICT the hand-rolled signature scheme is close to what the JWT token would already do?

this just allows the client to access its data by signing a message.

AFAICT we still don't couple store_id to any particular user ID though? So we'd currently only check that a user is authenticated, not what exactly it is authenticated for?

This sidesteps having to implement a second server entirely, not to mention reduces complexity and dependencies.

I had previously understood that the current idea is that we'd add issuance as part of the Rust codebase?

🔔 2nd Reminder

Hey @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Well, no, at the time there was some push-back to the idea of featuring a particular default/preferred authentication scheme, as some people were of the opinion that that should be completely left to the user as they would want to integrate with whatever authentication infrastructure they'd already have in place.

Sure, that's fine, but we can't complain that there isn't a single standard if we never tried to propose one because people wanted to do ad-hoc things?

Also not quite accurate, as we have users that have been running their LNURL-auth implementation for quite some time. But yes, we only recently finally made progress to add support to vss-server itself.

Right, my point is that its the only thing currently offered in our default implementation, not that people couldn't add their own auth.

I was under the impression that we concluded to finally change that and provide a default implementation?

I'm not at all convinced we should also go implement an LNURL-auth server on top of VSS and all the other stuff we built.

The point of having something like LNURL-Auth where there's a second server involved is that that other server does rate-limiting or whatever is required for user-signup validation. We can't readily provide a "default" for that as its highly application-dependent.

This PR proposes an alternative approach - for some users (certainly not all) they might do user-signup ratelimiting/validation at a higher level (eg via cookies or by embedding authentication tokens in the URI) and having a reverse proxy validate that authentication. Then, vss-server is only responsible for ensuring that the storage is tied to the seedphrase so that there's no mixing of data across wallets.

Not sure I'm completely following, AFAICT the hand-rolled signature scheme is close to what the JWT token would already do?
AFAICT we still don't couple store_id to any particular user ID though? So we'd currently only check that a user is authenticated, not what exactly it is authenticated for?

The JWT tokens are required to be signed by a fixed public key (a second server) which does application-specific user signup validation. The authentication scheme here is totally different - it relies on something else doing user signup validation and ties the store-id to the users' seedphrase (via some kind of static derivation path on the client side) rather than the higher-level validation.

For my use-case this is an important distinction because I have a Apple DeviceCheck service but that isn't authenticating to a specific store-id but rather only gating access (by having the client prove that it is a valid install of the app on real Apple hardware). The user might use several different seed phrases which would mean different store-ids, but to use LNURL-Auth I'd need yet another server, or have the vss-server sign auth tokens which is unnecessary complexity and an extra round-trip.

I think some of the confusion here is that "user authentication" may be totally distinct from store-id - storage needs to be tied to a specific wallet/seedphrase, but actual authentication might not be tied to that at all, which makes the LNURL-auth design not super helpful and just complexity for complexity's sake.

🔔 3rd Reminder

Hey @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

🔔 4th Reminder

Hey @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Good to squash with the CI fix

This PR proposes an alternative approach

Right, my main questions then are: if we think this is the better/simpler approach

a) should this maybe be the new default?
b) do we even want to support/maintain the JWT authorizer on the service side, or should we just double down on a single recommended default implementation that we drive to production-readiness as far as we can, rather than maintaining multiple half-baked implementations side-by-side?

a) should this maybe be the new default?

Code-wise imo yes, but that's because imo "just running it" should allow for a fully-functional (even if somewhat dos-prone) server. That doesn't necessarily imply that we suggest this as the way to do things in prod. For that I don't really have a great answer, sadly we do need some kind of account-creation-rate-limiting (as well as per-account limits #54). I suppose we could do that directly in the new authenticator in a followon PR (ie rate-limit auth with a fresh account-id based on X-Forwarded-For/client ip), though those ratelimits are likely to have to be materially too loose.

b) do we even want to support/maintain the JWT authorizer on the service side, or should we just double down on a single recommended default implementation that we drive to production-readiness as far as we can, rather than maintaining multiple half-baked implementations side-by-side?

I think that depends a bit on our users. If folks already have and want to keep LNURL-auth-based auth hopefully its not too much work to keep the existing jwt authorizer and not touch it too much more?

a) should this maybe be the new default?

I'm leaning towards yes; it's a self-standing solution that prevents unauthorized deletes.

b) do we even want to support/maintain the JWT authorizer on the service side, or should we just double down on a single recommended default implementation that we drive to production-readiness as far as we can, rather than maintaining multiple half-baked implementations side-by-side?

Shouldn't be too bad to maintain both no ? Especially if we decide not to provide a JWT issuance solution, and just provide the verification part.